Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show checkbox for sepa based payment methods to set allow_redisplay #4503

Merged
merged 16 commits into from
Feb 11, 2025

Conversation

wooj-stripe
Copy link
Collaborator

Summary

Adds the consent checkbox for sepa based payment methods when saving and customerSession.features.payment_method_save is set to enabled.

Motivation

allow_redisplay value is not being set for sepa based payment methods

Testing

existing tests & will add additional tests.

Changelog

@stripe stripe deleted a comment from github-actions bot Feb 4, 2025
@wooj-stripe wooj-stripe force-pushed the wooj/sepaAllowRedisplay branch from 61ebf8d to 553de95 Compare February 4, 2025 18:29
@wooj-stripe wooj-stripe force-pushed the wooj/sepaAllowRedisplay branch from dc175c4 to e7e31ea Compare February 5, 2025 01:59
@wooj-stripe wooj-stripe marked this pull request as ready for review February 5, 2025 19:42
@wooj-stripe wooj-stripe requested review from a team as code owners February 5, 2025 19:42
@wooj-stripe wooj-stripe force-pushed the wooj/sepaAllowRedisplay branch from eeffed4 to e7c2c53 Compare February 5, 2025 19:49
@@ -30,7 +30,7 @@ class PaymentSheetVerticalUITests: PaymentSheetUITestCase {
settings.mode = .setup
settings.customerMode = .new
settings.currency = .eur
settings.customerKeyType = .legacy // TODO: Change to customerSessions when adding save checkbox for SEPA
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also see a TODO like this in PaymentSheetStandardLPMUIOneTests.testSEPADebitPaymentMethod_PaymentSheet. Should we update that one too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YES! thank you!!

Comment on lines 634 to 637
format: STPLocalizedString(
"Save this account for future %@ payments",
"Prompt next to checkbox to save bank account."
),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this string and comment hardcoded twice in this file, should we move it to String.Localized?

porter-stripe
porter-stripe previously approved these changes Feb 11, 2025
@wooj-stripe wooj-stripe enabled auto-merge (squash) February 11, 2025 21:09
@wooj-stripe wooj-stripe merged commit 1991002 into master Feb 11, 2025
6 checks passed
@wooj-stripe wooj-stripe deleted the wooj/sepaAllowRedisplay branch February 11, 2025 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants